Skip to content

Add optional depth parameter to SDK file listing methods. #649

Merged
0div merged 35 commits intomainfrom
recursive-files-list-in-the-sdk-e2b-1902_2
May 5, 2025
Merged

Add optional depth parameter to SDK file listing methods. #649
0div merged 35 commits intomainfrom
recursive-files-list-in-the-sdk-e2b-1902_2

Conversation

@0div
Copy link
Copy Markdown
Contributor

@0div 0div commented Apr 1, 2025 •
edited
Loading

Description

Do not merge before e2b-dev/infra#524 is merged and deployed.

The DX would look like this:

const files = await sandbox.files.list(dirName, { depth: 3 })
  • update envd pb spec
  • generate for js-sdk
  • update js-sdk file.list method to include optional depth param
  • update js-sdk tests
  • generate for python-sdk
  • update python-sdk file.list method to include optional depth param
    • sync
    • async
  • update python-sdk tests
    • sync
    • async

@0div
* update envd spec & generate for js-sdk
db5089c 
* update js-sdk file.list method to include optional depth param
* update js-sdk tests
@0div 0div added the feature New feature or request label Apr 1, 2025
@0div 0div self-assigned this Apr 1, 2025
@linear
Copy link
Copy Markdown

linear bot commented Apr 1, 2025

E2B-1902 Recursive files list in the SDK

@changeset-bot
Copy link
Copy Markdown

changeset-bot bot commented Apr 1, 2025 •
edited
Loading

⚠️ No Changeset found

Latest commit: d7f859c

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@0div 0div mentioned this pull request Apr 1, 2025
Merged
5 tasks
@ValentaTomas
Copy link
Copy Markdown
Member

ValentaTomas commented Apr 2, 2025

Also, add docstrings for the new depth parameter.

I'll document the spec update process so it is not blocking.

@ValentaTomas
Copy link
Copy Markdown
Member

ValentaTomas commented Apr 3, 2025

This is blocked by the unpredictable Python client generation, right?

@0div
Copy link
Copy Markdown
Contributor Author

0div commented Apr 3, 2025

This is blocked by the unpredictable Python client generation, right?

@ValentaTomas yes, per discussion, im on a side-quest to build a predictable codegen, will apply it here and commit.

0div and others added 3 commits April 3, 2025 17:03
@0div
* create Docker image with all the pinned deps needed to codegen
8e9ea77 
* create codegen script using this container with mapped volumes
* adapt Makefiles to this new approach
* adapt where we install the connect-python protobuf binary for this to
  work
@0div
Merge branch 'main' into make-codegen-deterministic-for-sdks-e2b-1969
66d213a
@0div
:wMerge branch 'main' of https://github.com/e2b-dev/E2B into recursiv…
181cd51 
…e-files-list-in-the-sdk-e2b-1902_2
@ValentaTomas ValentaTomas requested a review from mishushakov April 7, 2025 19:03
@0div 0div marked this pull request as ready for review April 14, 2025 18:50
mishushakov
mishushakov requested changes Apr 14, 2025
View reviewed changes
Copy link
Copy Markdown
Member

@mishushakov mishushakov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks fine, the only things I am questioning if we should hardcode the default depth (any other examples where we hardcode a parameter like this?) and if we can reduce some repetition in the tests (using loops?)

Comment thread packages/js-sdk/src/sandbox/filesystem/index.ts Outdated
Comment thread packages/js-sdk/src/sandbox/filesystem/index.ts
Comment thread packages/js-sdk/tests/integration/next.test.ts Outdated
Comment thread packages/js-sdk/tests/sandbox/files/list.test.ts Outdated
Comment thread packages/js-sdk/tests/sandbox/files/list.test.ts
Comment thread packages/js-sdk/tests/sandbox/files/list.test.ts Outdated
Comment thread packages/python-sdk/e2b/sandbox_async/filesystem/filesystem.py
Comment thread packages/python-sdk/e2b/sandbox_sync/filesystem/filesystem.py
Comment thread packages/python-sdk/tests/async/sandbox_async/files/test_files_list.py Outdated
Comment thread packages/python-sdk/tests/sync/sandbox_sync/files/test_files_list.py Outdated
jakubno
jakubno reviewed Apr 30, 2025
View reviewed changes
Comment thread packages/js-sdk/tests/sandbox/files/list.test.ts
@0div 0div requested a review from jakubno May 1, 2025 21:29
@0div 0div merged commit a5b08de into main May 5, 2025
7 checks passed
@0div 0div deleted the recursive-files-list-in-the-sdk-e2b-1902_2 branch May 5, 2025 16:17
@0div 0div mentioned this pull request May 5, 2025
Merged
jakubno pushed a commit that referenced this pull request May 5, 2025
@0div
Add minor changeset to python and js SDKs (#716)
2703f75 
`npx changeset` for the release of
#649
This was referenced May 5, 2025
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mishushakov mishushakov mishushakov approved these changes

@jakubno jakubno jakubno approved these changes

@ValentaTomas ValentaTomas Awaiting requested review from ValentaTomas ValentaTomas is a code owner

Assignees

@mishushakov mishushakov

@0div 0div

Labels

feature New feature or request

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@0div @ValentaTomas @mishushakov @jakubno